Skip to content

fix: avoid rendering share modal without a session - #13942

Open
KRRT7 wants to merge 4 commits into
warpdotdev:masterfrom
KRRT7:fix-share-modal-render-loop
Open

fix: avoid rendering share modal without a session#13942
KRRT7 wants to merge 4 commits into
warpdotdev:masterfrom
KRRT7:fix-share-modal-render-loop

Conversation

@KRRT7

@KRRT7 KRRT7 commented Jul 18, 2026

Copy link
Copy Markdown
Contributor

Description

Fixes a share-modal lifecycle bug where the pane group could mark the modal open before confirming the terminal session existed. That left a modal with no model mounted, causing repeated render-time warnings and unnecessary repaint/logging work.

Linked Issue

Testing

  • cargo nextest run -p warp -E "test(share_block_modal)"
  • cargo clippy -p warp --lib --tests -- -D warnings
  • cargo fmt --all
  • git diff --check

This is a lifecycle/performance fix with no visual UI change, so screenshots are not applicable.

Changelog

CHANGELOG-BUG-FIX: Avoid repeated share-modal rendering when no terminal session is available.

@cla-bot cla-bot Bot added the cla-signed label Jul 18, 2026
@github-actions github-actions Bot added the external-contributor Indicates that a PR has been opened by someone outside the Warp team. label Jul 18, 2026
@oz-for-oss

oz-for-oss Bot commented Jul 18, 2026

Copy link
Copy Markdown
Contributor

@KRRT7

I'm starting a first review of this pull request.

You can view the conversation on Warp.

I reviewed this pull request and requested human review from: @seemeroland.

Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).

Powered by Oz

@oz-for-oss oz-for-oss Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overview

This PR prevents the pane group from marking the share-block modal open until it has confirmed that the terminal session still exists, and quiets the render-time fallback log for stale modal/model races.

Concerns

  • No blocking correctness, security, or spec-alignment concerns found in the changed hunks.

Verdict

Found: 0 critical, 0 important, 0 suggestions

Approve

Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).

Powered by Oz

@oz-for-oss
oz-for-oss Bot requested a review from seemeroland July 18, 2026 12:27

@seemeroland seemeroland left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change looks good, but I would be surprised if this resolves the problem. I suspect there's some other case we're rendering the modal when we shouldn't

Comment thread app/src/terminal/share_block_modal.rs Outdated
@KRRT7

KRRT7 commented Jul 21, 2026

Copy link
Copy Markdown
Contributor Author

@seemeroland agreed, there's other issues as well (check my other PRs) but I started small since my PRs typically stall here.

KRRT7 and others added 3 commits July 31, 2026 14:30
@KRRT7

KRRT7 commented Jul 31, 2026

Copy link
Copy Markdown
Contributor Author

/oz-review

@oz-for-oss

oz-for-oss Bot commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

@KRRT7

I'm re-reviewing this pull request in response to a review request.

You can view the conversation on Warp.

I reviewed this pull request and requested human review from: @seemeroland.

Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).

Powered by Oz

@oz-for-oss oz-for-oss Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overview

This PR prevents the share-block modal from being marked open when the terminal session cannot be resolved, and it clears stale share-modal state before close-pane early returns. It also adds a regression test for the last-pane close path.

Concerns

  • No blocking correctness, security, or spec-drift concerns found in the reviewed diff.

Verdict

Found: 0 critical, 0 important, 0 suggestions

Approve

Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).

Powered by Oz

@oz-for-oss
oz-for-oss Bot requested a review from seemeroland July 31, 2026 19:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla-signed external-contributor Indicates that a PR has been opened by someone outside the Warp team.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Tried to render share modal without a model fires in a ~10-min render loop (up to 84/sec)

2 participants